Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

4조 과제 제출(유지석, 소재헌, 윤준수, 우지수, 임예지) #3

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Jaeheon-So
Copy link
Member

Netlify Status


프로젝트 소개

유튜브 API를 활용한 유튜브 클론

배포 사이트

불사조 유튜브

팀원

팀장: 유지석 팀원: 소재헌 팀원: 윤준수 팀원: 우지수 팀원: 임예지

🔨기술 스택



📆 과제 기간 및 담당 업무

과제 기간: 2023. 1. 16 ~ 2022. 1. 20

  • 유지석 : <상세 페이지> 동영상 설명, 댓글
  • 소재헌 : <상세 페이지> 동영상 상세 정보
  • 윤준수 : <상세 페이지> 관련동영상
  • 우지수 : <메인 페이지, 사이드바> 메인 페이지 전체, 사이드바
  • 임예지 : <검색 페이지, 헤더> 검색 페이지 전체, 헤더


🎈기능 구현

  • 메인 페이지에서는 10개의 고정된 동영상을 보여준다.
  • 검색 페이지에서는 검색어에 해당하는 10개의 동영상을 보여준다.(채널은 제외)
  • 동영상을 클릭하면 상세 페이지로 이동한다.
  • 상세 페이지에서는 해당 동영상의 영상, 정보, 설명, 댓글, 관련동영상을 보여준다.
  • 사용자의 컴퓨터 설정에 따른 다크, 라이트 모드를 적용시켰다.
  • 검색 페이지와 헤더를 제외한 나머지 부분은 반응형으로 만들었다.
  • api 할당량을 초과하면 고정된 더미 데이터를 보여준다.


🔔어려웠던 점

  • 타입스크립트가 익숙하지 않아서 타입 오류를 해결하는 것이 많이 힘들었다.
  • 중복되는 함수들을 합치는 작업이 어려웠다.
  • api 호출 할당량이 적어서 실제로 되는지 안되는지 확인하는 것이 힘들었다.
  • 반응형 작업이 어려웠다.


💡궁금한 점

  • api 호출시, 여러 이펙트안에서 분리해서 각각 호출하는 것이 좋은지
  • 하나의 이펙트안에서 호출하는 게 좋은지
  • 혹은 하나의 이펙트안에서 axios.all을 이용해 처리하는게 좋은지
  • 자녀 컴포넌트에서 각자 api 호출이 좋은지, 부모 컴포넌트에서 호출을 한 후 prop으로 내리는 게 좋을지
  • 더미데이터를 useState의 기본값에 할당을 해주어서 처음 렌더링 할때 더미데이터 값이 먼저 렌더링 되고 그 다음에 api에서 받아온 값이 렌더링 되는데 고칠 수 있는 방법이 있는지

Copy link

@sangheon-kim sangheon-kim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

전체적으로 고생하셨고, 타입스크립트 사용하시려고 하신것도 인상깊게 봤습니다. 추가팁은 명시해드렸습니다

슬랙에 공통으로 고지드린 내용은 매번 코멘트는 생략했습니다


useEffect(() => {
window.onbeforeunload = function pushRefresh() {
window.scrollTo(0, 0);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

신박하긴하네요 beforeunload에 pushRefresh를 넣어준 방식은 근데 return () => {} 형식으로 언마운트 또는 변경 이전에 window.beforeunload = null 해주면 좋겠어요

const App = () => {
const [open, setOpen] = useState(false);

const handleClickOpen = () => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

React.useCalback 사용해서 최적화 시도해보세요
지금 함수 내용을 보니 마운트지점에 조치해주면될 것 같습니다

@@ -0,0 +1,8 @@
import axios from "axios";

export const instance = axios.create({

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그냥 instance보다 baseInstanc 등의 이름을 명시해보세요 :) 그러면 나중에 추후에 인증서버와 작업하거나 할때 인스턴스를 export하실때 훨씬 확장성에 좋을 것 같아요

그리고 create 내부에 들어가는 것들중 중복되는건 별도 객체로 만든 후 spread Operator와 결합해보세요

setError: React.Dispatch<React.SetStateAction<string>>,
): void;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

보통 fetcher에서는 data 값을 반환해주고, 해당 fetcher를 사용하는쪽에서 주로 행위를 해주는데, 신박하네요

흠... res.data를 빼주고, 차라리 정확하게 response Type을 명시해서 Promise을 명시해주면 어떨지 생각해봅니다.

}, []);

// // dummy data 로컬에 저장
localStorage.setItem(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

렌더 시점마다 호출할건가요?
뭔가 useEffect를 넣든 처리가 필요해보입니다.

totalResults: number;
resultsPerPage: number;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

declaration Type을 명시해두고 export를 하는 것은 좋지 않습니다 typeRoots설정해서 exrpot 분리하시고, import없이 사용하세요

url: string
width: number
height: number
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위에서 말씀드린대로, export 제거해주세요

window.scrollTo(0, 0);
}, [pathname]);
return null;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

util내에서 커스텀훅을 쓰면 에러나 경고나올텐데요 사용처에 따라 달라질 수 있으니 이런경우에는 해당 유틸을 커스텀훅으로 만들어서 사용하세요

커스텀훅으로 사용시에 명명은 use로 사용하지 않으면 에러나 경고문구가 출력될겁니다. src/hooks로 담아보세요

}

return `${Math.floor(betweenTimeDay / 365)}년 전`;
};

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

util명을 함수별로 따시지말고 기능별로 따면 좋을 것 같아요. 단위 관련유틸 시간관련 유틸 이렇게요~

}
}
}
};

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이런건 videoUtil이 좋겠죠?

@sangheon-kim
Copy link

어려웠던 점
타입스크립트가 익숙하지 않아서 타입 오류를 해결하는 것이 많이 힘들었다.
중복되는 함수들을 합치는 작업이 어려웠다.
api 호출 할당량이 적어서 실제로 되는지 안되는지 확인하는 것이 힘들었다.
반응형 작업이 어려웠다.

-> 어려우셨지만, 위 노력을 통해 많은 경험을 얻으셨을 것 같습니다.

💡궁금한 점
api 호출시, 여러 이펙트안에서 분리해서 각각 호출하는 것이 좋은지 하나의 이펙트안에서 호출하는 게 좋은지
-> 여러 이펙트안이 어떤 말씀이신지 잘 모르곘습니다. useEffect를 말씀하시는건지?
-> useEffect안에 subscription 항목을 확인하셔서 호출하면 되지않을까요?

혹은 하나의 이펙트안에서 axios.all을 이용해 처리하는게 좋은지
자녀 컴포넌트에서 각자 api 호출이 좋은지, 부모 컴포넌트에서 호출을 한 후 prop으로 내리는 게 좋을지
-> 이런 부분때문에 주로 전역상태에 담아두고 store에서 댕겨오는 방식을 쓰기도 헀고, 때에 따라 react-query를 사용해서 해당 데이터가 필요한 부분에서 내려주기도 합니다. 그리고 보통 자식 컴포넌트를 순수 UI컴포넌트로 활용할거라면 부모컴포넌트에서 모든 상태로직을 갖고있는게 좋죠
-> 보통은 그래서 비즈니스로직 관련해서는 이런고민떄문에 데이터 관련해서는 별도의 커스텀훅을 만들어서 사용하는 방식을 씁니다.

더미데이터를 useState의 기본값에 할당을 해주어서 처음 렌더링 할때 더미데이터 값이 먼저 렌더링 되고 그 다음에 api에서 받아온 값이 렌더링 되는데 고칠 수 있는 방법이 있는지
-> 더미데이터를 넣지 않고, 처음에는 empty값으로 넣어서 empty에 대한 처리를 해두면 되지않나요? 아니면 해당 컴폰너트를 데이터 값에 따라 렌더링 할지 말지 결정하면 될 것 같습니다

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants